Skip to content

fix(kotlin-mutex): name the coroutine behind a tryLock from its enclosing continuation (#1220) - #1226

Merged
PIsberg merged 2 commits into
mainfrom
fix/1220-mutex-trylock
Nov 8, 2026
Merged

PIsberg merged 2 commits into
mainfrom
fix/1220-mutex-trylock

Conversation

@PIsberg

@PIsberg PIsberg commented Nov 8, 2026

Copy link
Copy Markdown
Owner

Stacked on #1225: review the last commit (91bab9297). The first commit is #1225's and drops out of this diff once that merges. Both change docs/CHANGELOG.md, which is why they are chained.

Partly addresses #1220. select { onLock } stays open there (narrowed in a comment on the issue).

Why

A Mutex taken with tryLock held no owner token of the agent's. tryLock takes no continuation, so the hook could not read the caller's Job. A coroutine that took a mutex with tryLock and then locked it again from a coroutineScope or supervisorScope block hung exactly like the case the detector reports, and nothing was reported. The new fixture showed this before the change: Findings were: [].

What changed

  • Weaver. Before a tryLock hook, KotlinMutexWeaver loads the continuation of the function the call is made in:

    • ALOAD 0 in an instance invokeSuspend(Object), the body of a suspend lambda;
    • an ALOAD of a trailing Continuation parameter, in a suspend function;
    • ACONST_NULL anywhere else.

    The hook takes it as one more argument, so the stack grows by one and frames are unchanged (COMPUTE_MAXS). A slot the method stores a non-reference value into is not loaded, so verification cannot fail. javac and kotlinc never do that to a parameter.

  • Hooks. The new kotlinMutexTryLockIn and kotlinMutexTryLockDefaultIn make the call with a fresh owner token when none was passed, as the lock hook does since [P3] Kotlin Mutex detector keeps a stale holder after an unlock in an unwoven class and can report a false reentrancy #1221. They record a mutex they take as held by the caller's Job. A later lock by that coroutine that waits is then reported by the existing rule. The old kotlinMutexTryLock and kotlinMutexTryLockDefault hooks stay, so an older agent jar against this library still links. Both new hooks are additive public API, @since 1.14.2.

  • Guardrail. The @AICore note on AsyncTestAgent, which inventories every weaver's bytecode shape, now describes the extra load. The guardrail files were regenerated by the build.

How it was verified

  • Tests first: KotlinMutexReentrancyAfterTryLockIsReportedTest, against real kotlinx 1.11 in consumer-fixture-langs/kotlin, failed before the change with no findings. It now passes for a tryLock in a runBlocking lambda (the invokeSuspend path) and for one in a named suspend function (the parameter path).
  • Silent twin: tryLock, unlock, then lock by the same coroutine stays silent. The 10 existing Kotlin mutex cases, reported and silent, stay green (12/12 in the module).
  • KotlinMutexWeaverTest pins the slot rule: this in invokeSuspend, a trailing parameter after a two-slot long, nothing for a static invokeSuspend or an ordinary function. The row-descriptor test now covers the extra argument.
  • Library suite 3963 green, agent suite 118 green (-P fast). The static gate chain (mvn install -DskipTests -Djacoco.skip=true) is green and regenerated the guardrails.

Docs

docs/CHANGELOG.md, docs/JVM_LANGUAGES.md, docs/agent/attaching.md, the catalog entry in docs/detector-catalog/12-futures-scale-structured.md, and the KotlinMutexReentrancyDetector javadoc all said a tryLock holds no token. They now say what still holds none: select, and a tryLock outside any suspend function. The VibeTags change is limited to the AsyncTestAgent note above, because a new bytecode shape is exactly what that note exists to list. No new annotation was added.

Checklist

  • A behaviour change ships with a test that fails without the fix
  • A new or changed detector is tested against the buggy code and its correctly synchronized twin
  • Docs the change makes wrong are updated in this same change
  • Nothing was hand-edited between VIBETAGS-START / VIBETAGS-END markers (the annotation changed, the build regenerated)
  • Public API changes are intentional and carry the right version bump: 2 additive hooks, @since 1.14.2
  • Commits an agent authored carry the Co-Authored-By trailer
  • A new dependency was proposed with a reason (none added)
  • CI is green, or the failure is explained below

Still missed (tracked in #1220)

select { mutex.onLock { } }, and a tryLock made outside any suspend function, where there is no coroutine to name.

🤖 Generated with Claude Code

PIsberg and others added 2 commits November 8, 2026 14:48
…allel pipeline (#1172)

#1209 followed a reference Stream's map, filter and peek lambdas to the
common pool's workers, but a primitive stream's were still unattributed,
and so was any lambda behind mapToInt. list.parallelStream().mapToInt(...)
.sum() is the usual way to total in parallel, and a plain counter inside
its lambda was seen only on the caller's own elements.

IntStream, LongStream and DoubleStream now get the Stream treatment: map,
filter, peek and mapToObj wait in a slot, and the terminal operations
(forEachOrdered, reduce, collect, the matches, sum, count, average, min,
max, summaryStatistics, toArray, the finds) lend the run to every slot
while they run. The pipeline map is keyed by BaseStream, so the slots
follow a pipeline across Stream.mapToInt/mapToLong/mapToDouble, mapToObj
and boxed. The three primitive forEach hooks now share parallelTerminal.

Tests first: six new cases in AgentParallelStreamHooksTest failed to
compile on the 59 missing hooks; all 19 pass. Disabling the mapToInt hook
turns two of them red. The class now runs a telemetry drain per test:
without one the ring filled and every later task event waited out the
ring's one-second give-up, which hung the class for over 15 minutes.

Corpus: four idioms, IntStream.map and Stream.mapToInt counters with and
without a lock; idiom lane 86/86, every lane green. Library 3963 and agent
117 tests green, static gate chain green.

Still not followed: a stage behind sorted, distinct, limit, skip or
flatMap, a pipeline ended by iterator(), and a Collector's own functions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sing continuation (#1220)

A Mutex taken with tryLock held no owner token of the agent's, because
tryLock takes no continuation and the hook could not read the caller's
Job. A coroutine that tryLocked a mutex and then, from a coroutineScope or
supervisorScope block, locked it again hung exactly like the reported
case, and nothing was reported (Findings were: []).

The weaver now loads the continuation of the function a tryLock is made
in before its hook: this in a suspend lambda's invokeSuspend, the trailing
Continuation parameter of a suspend function, or null elsewhere. A slot
the method stores a non-reference into is not loaded, so verification
cannot fail. The new kotlinMutexTryLockIn and kotlinMutexTryLockDefaultIn
hooks make the call with a fresh owner token when none was passed, as the
lock hook does, and record a mutex they take as held by that Job. The old
two tryLock hooks stay for an older agent.

Tests first: KotlinMutexReentrancyAfterTryLockIsReportedTest, against real
kotlinx in consumer-fixture-langs/kotlin, failed before the change with no
findings; it passes now for a tryLock in a runBlocking lambda and in a
named suspend function. The silent twin (tryLock, unlock, then lock) stays
silent, and the 10 existing mutex cases stay green. KotlinMutexWeaverTest
pins the slot rule. Library 3963 and agent 118 tests green, static gate
chain green, guardrails regenerated from the AsyncTestAgent note.

Still missed: select { onLock }, and a tryLock outside any suspend function.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov

codecov Bot commented Nov 8, 2026

Copy link
Copy Markdown

@PIsberg
PIsberg merged commit a4b5da5 into main Nov 8, 2026
55 checks passed
@PIsberg
PIsberg deleted the fix/1220-mutex-trylock branch November 8, 2026 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant